Allow the use of custom CA Certificates#162
Allow the use of custom CA Certificates#162GauntletWizard wants to merge 4 commits intoauthzed:mainfrom
Conversation
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
GauntletWizard
force-pushed
the
ted/cacert
branch
from
276b3a0 to
c42c593
Compare
|
I have read the CLA Document and I hereby sign the CLA |
Signed-off-by: Thomas Hahn <Thahn@tcbtech.com>
GauntletWizard
force-pushed
the
ted/cacert
branch
from
c42c593 to
576b769
Compare
jzelinskie
left a comment
jzelinskie
left a comment
There was a problem hiding this comment.
I think --certificate-path might be the best name for this flag.
| } | ||
|
|
||
| if cobrautil.MustGetBool(cmd, "insecure") && cobrautil.MustGetString(cmd, "cafile") != "" { | ||
| panic("cafile flag cannot be combined with insecure") |
There was a problem hiding this comment.
This should definitely be checked somewhere else and return an error rather than panicking.
There was a problem hiding this comment.
I don't seen an obvious place to put a "verify flags" precondition; I'm not that familiar with Cobra
There was a problem hiding this comment.
Yeah, that's pretty fair -- I think we should just ignore the CA flag if the insecure flag is set. The insecure flag description is "connect over a plaintext connection", so if you specify it, that's what we should do.
The real problem is that the grpcutil library panics if it cannot find a CA rather than returning an error.
I'll fix things upstream to improve the situation.
|
Closed in favor of #183. Let me know if there's anything missing so that we can follow-up. Thanks again for pushing this forward. |
2 participants
This allows the use of a custom CA Pool/TrustBundle via the
cafileflag. Starts on #161, but does not support contexts, which would presumably want to embed the ca pool in the context secret.The decision to use
cafilefor the flag name was based oncurl, but kubectl calls it--certificate-authorityand I've seen others.